fix: reduce souls browse overfetch by ImLukeF · Pull Request #1637 · openclaw/clawhub

ImLukeF · 2026-04-12T10:51:36Z

Summary

add the missing souls.by_active_updated index so browse queries can target active souls directly
update souls.list browse mode to read limit active rows instead of limit * 5 rows filtered in JS
add focused regression coverage for the browse-path query shape
rename a shadowed local in src/routes/cli/auth.tsx so lint passes cleanly

Why

The previous browse listing path over-read soul documents and then filtered soft-deleted rows in application code. That meant requests like /souls with limit=500 could read up to 2500 rows to return 500 visible items.

This patch keeps the behavior the same while making the browse path ask Convex for active rows directly.

Verification

bun run lint
bun x vitest run convex/souls.test.ts src/routes/cli/-auth.test.ts
bun run build

Notes

This supersedes the approach in fix: use indexed query for soul list to avoid over-fetching #1531 with a smaller patch that includes the missing schema index.
bunx convex codegen is blocked in this shell because no CONVEX_DEPLOYMENT is configured here.

vercel · 2026-04-12T10:51:44Z

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
clawhub	Ready	Preview, Comment	Apr 12, 2026 10:51am

Copilot

Pull request overview

This PR reduces over-fetching in the /souls browse listing by adding and using an index that targets active (non-soft-deleted) souls directly, and adds a focused regression test to ensure the optimized query shape is preserved.

Changes:

Add souls.by_active_updated compound index (softDeletedAt, updatedAt) to support efficient active-only browsing.
Update souls.list browse mode to use the new index and take(limit) instead of take(limit * 5) with JS-side filtering/slicing.
Add a regression test asserting the browse query uses the active index and respects the requested limit; rename a shadowed local in the CLI auth route.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

File	Description
src/routes/cli/auth.tsx	Renames a shadowed local variable used for redirect URL construction to satisfy linting.
convex/souls.ts	Switches browse listing to an active-only indexed query and removes JS-side overfetch filtering.
convex/souls.test.ts	Adds regression coverage to assert browse listing uses the active index and only takes `limit`.
convex/schema.ts	Adds the missing `by_active_updated` index to the `souls` table schema.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

greptile-apps · 2026-04-12T11:01:46Z

Greptile Summary

This PR fixes the souls.list browse path to use the new by_active_updated compound index (["softDeletedAt", "updatedAt"]) so Convex returns only active souls directly, replacing the previous take(limit * 5) over-fetch and JS filter.

The .order(\"desc\") on the new index sorts by updatedAt (the second index field after softDeletedAt is equality-filtered), not by _creationTime as the old scan did — a silent ordering change that contradicts the PR's claim of identical behaviour.

Confidence Score: 4/5

Safe to merge once the sort-order change is acknowledged or corrected — no data loss or security risk, but browsing order changes silently.

One P1 finding: the browse listing's sort order silently changes from _creationTime to updatedAt. This is a real behavioural difference that the PR description claims doesn't exist. If the change is intentional (aligning with listPublicPage), confirming that and documenting it would make this ready to merge at 5/5.

convex/souls.ts — browse path ordering and owner-path over-fetch

Comments Outside Diff (1)

convex/souls.ts, line 127-137 (link)

Owner-scoped path still over-fetches

The ownerUserId branch still reads limit * 5 rows and filters soft-deleted ones in JS — the exact pattern this PR fixes for the browse path. A compound index like ["ownerUserId", "softDeletedAt", "updatedAt"] would let this branch take limit rows directly too. Out of scope here, but worth a follow-up.

Prompt To Fix With AI

This is a comment left during a code review.
Path: convex/souls.ts
Line: 127-137

Comment:
**Owner-scoped path still over-fetches**

The `ownerUserId` branch still reads `limit * 5` rows and filters soft-deleted ones in JS — the exact pattern this PR fixes for the browse path. A compound index like `["ownerUserId", "softDeletedAt", "updatedAt"]` would let this branch take `limit` rows directly too. Out of scope here, but worth a follow-up.

How can I resolve this? If you propose a fix, please make it concise.

Prompt To Fix All With AI

This is a comment left during a code review.
Path: convex/souls.ts
Line: 139-143

Comment:
**Silent sort-order change: `_creationTime` → `updatedAt`**

The old scan (no index, `.order("desc")`) sorted by `_creationTime` — Convex's default ordering. The new `by_active_updated` index is `["softDeletedAt", "updatedAt"]`, so after the equality constraint on `softDeletedAt` the remaining sort key is `updatedAt`. The browse listing will now return souls ordered by most-recently-*updated* rather than most-recently-*created*, which is a silent behavioral change. The PR description says "keeps the behavior the same," but the sort order is different. If this is intentional (aligning with `listPublicPage`'s `by_updated` ordering), please call it out explicitly in the description.

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: convex/souls.ts
Line: 127-137

Comment:
**Owner-scoped path still over-fetches**

The `ownerUserId` branch still reads `limit * 5` rows and filters soft-deleted ones in JS — the exact pattern this PR fixes for the browse path. A compound index like `["ownerUserId", "softDeletedAt", "updatedAt"]` would let this branch take `limit` rows directly too. Out of scope here, but worth a follow-up.

How can I resolve this? If you propose a fix, please make it concise.

_{Reviews (1): Last reviewed commit: "fix: reduce souls browse overfetch" | Re-trigger Greptile}

greptile-apps · 2026-04-12T11:01:49Z

convex/souls.ts

    const entries = await ctx.db
      .query("souls")
+      .withIndex("by_active_updated", (q) => q.eq("softDeletedAt", undefined))
      .order("desc")
-      .take(limit * 5);
+      .take(limit);


Silent sort-order change: _creationTime → updatedAt

The old scan (no index, .order("desc")) sorted by _creationTime — Convex's default ordering. The new by_active_updated index is ["softDeletedAt", "updatedAt"], so after the equality constraint on softDeletedAt the remaining sort key is updatedAt. The browse listing will now return souls ordered by most-recently-updated rather than most-recently-created, which is a silent behavioral change. The PR description says "keeps the behavior the same," but the sort order is different. If this is intentional (aligning with listPublicPage's by_updated ordering), please call it out explicitly in the description.

Prompt To Fix With AI

This is a comment left during a code review. Path: convex/souls.ts Line: 139-143 Comment: **Silent sort-order change: `_creationTime` → `updatedAt`** The old scan (no index, `.order("desc")`) sorted by `_creationTime` — Convex's default ordering. The new `by_active_updated` index is `["softDeletedAt", "updatedAt"]`, so after the equality constraint on `softDeletedAt` the remaining sort key is `updatedAt`. The browse listing will now return souls ordered by most-recently-*updated* rather than most-recently-*created*, which is a silent behavioral change. The PR description says "keeps the behavior the same," but the sort order is different. If this is intentional (aligning with `listPublicPage`'s `by_updated` ordering), please call it out explicitly in the description. How can I resolve this? If you propose a fix, please make it concise.

ImLukeF · 2026-04-12T11:04:07Z

Thanks. I checked the ordering concern before merging.

For /souls, the route fetches the browse set and then applies client-side sorting, so the server-side browse order is not what drives that screen. Separately, the paginated public listing path already uses by_updated, so using the active+updated index here is acceptable and consistent with the broader soul listing direction.

I’m treating the owner-branch overfetch as follow-up scope, not a blocker for this patch.

@momothemage

* build(deps-dev): bump vite in the npm_and_yarn group across 1 directory (#1561) Bumps the npm_and_yarn group with 1 update in the / directory: [vite](https://github.com/vitejs/vite/tree/HEAD/packages/vite). Updates `vite` from 8.0.1 to 8.0.5 - [Release notes](https://github.com/vitejs/vite/releases) - [Changelog](https://github.com/vitejs/vite/blob/main/packages/vite/CHANGELOG.md) - [Commits](https://github.com/vitejs/vite/commits/v8.0.5/packages/vite) --- updated-dependencies: - dependency-name: vite dependency-version: 8.0.5 dependency-type: direct:development dependency-group: npm_and_yarn ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * fix: detect generated-source template injection in skill scans (#1597) * fix: detect exposed resource identifiers in skill scans (#1598) * fix: restore ci checks after lockfile drift * refactor: address actionable review cleanup (#1601) * fix: prevent starring soft-deleted skills and fix star count reconciliation (#1605) * feat: Add support for Chinese Japanese and Korean(CJK) skills search (#1596) Merged via squash. Prepared head SHA: ab58f01 Co-authored-by: pq-dong <[email protected]> Co-authored-by: momothemage <[email protected]> Reviewed-by: @momothemage * docs: document CLI config paths across platforms (#1252) * docs: document CLI config paths across platforms * docs: clarify legacy config fallback --------- Co-authored-by: ImLukeF <[email protected]> * fix: point plugin metadata help link to OpenClaw docs (#1399) * fix: point plugin metadata help link to OpenClaw docs * fix: open plugin metadata docs in a new tab * fix(cli-auth): ensure fallback token renders before redirect on Windows/Chrome (#1486) * fix(cli-auth): ensure fallback token renders before redirect on Windows/Chrome React batches state updates, so setToken() and window.location.assign() previously raced: the navigation could fire before React re-rendered the fallback token UI. On Chrome/Windows this means a failed http:// redirect (ERR_CONNECTION_REFUSED, HTTPS-first interference) would replace the page with an error screen before the user ever saw the token. Use flushSync() to render the token synchronously, then attempt window.location.assign(). If the redirect fails the token and a "Retry redirect to CLI" link are already painted on screen. Fixes #1469 Co-Authored-By: Claude Sonnet 4.6 <[email protected]> * test: cover cli auth fallback redirect --------- Co-authored-by: Claude Sonnet 4.6 <[email protected]> Co-authored-by: ImLukeF <[email protected]> * fix: reduce souls browse overfetch (#1637) * fix: improve admin user search coverage (#1466) * fix admin user search coverage * fix admin user search without full table scan * feat: include stats in package detail API response Expose package detail stats through the shared API contract and the app client. This lands the original package detail stats work and folds in the follow-up cleanup to keep the response shape sourced from the shared schema instead of a hand-maintained app-local type. Co-authored-by: Saurabh Jain <[email protected]> * test: cover package detail stats response * fix: normalize misleading MIME types for text files * feat: modernize clawhub app store --------- Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Luke <[email protected]> Co-authored-by: Momo <[email protected]> Co-authored-by: pqdong <[email protected]> Co-authored-by: Jholly <[email protected]> Co-authored-by: loong <[email protected]> Co-authored-by: Yaovi <[email protected]> Co-authored-by: Claude Sonnet 4.6 <[email protected]> Co-authored-by: Saurabh Jain <[email protected]>

fix: reduce souls browse overfetch

93f2187

Copilot AI review requested due to automatic review settings April 12, 2026 10:51

ImLukeF mentioned this pull request Apr 12, 2026

fix: use indexed query for soul list to avoid over-fetching #1531

Closed

vercel bot deployed to Preview – clawhub April 12, 2026 10:51 View deployment

Copilot started reviewing on behalf of ImLukeF April 12, 2026 10:52 View session

Copilot AI reviewed Apr 12, 2026

View reviewed changes

greptile-apps bot reviewed Apr 12, 2026

View reviewed changes

ImLukeF merged commit a28d014 into main Apr 12, 2026
11 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix: reduce souls browse overfetch#1637

fix: reduce souls browse overfetch#1637
ImLukeF merged 1 commit intomainfrom
imlukef/pr-1531

ImLukeF commented Apr 12, 2026

Uh oh!

vercel bot commented Apr 12, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

greptile-apps bot commented Apr 12, 2026 •

edited

Loading

Comments Outside Diff (1)

Uh oh!

greptile-apps bot Apr 12, 2026

Uh oh!

ImLukeF commented Apr 12, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

ImLukeF commented Apr 12, 2026

Summary

Why

Verification

Notes

Uh oh!

vercel bot commented Apr 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

greptile-apps bot commented Apr 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Greptile Summary

Confidence Score: 4/5

Comments Outside Diff (1)

Uh oh!

greptile-apps bot Apr 12, 2026

Choose a reason for hiding this comment

Uh oh!

ImLukeF commented Apr 12, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

vercel bot commented Apr 12, 2026 •

edited

Loading

greptile-apps bot commented Apr 12, 2026 •

edited

Loading